fix(backend): fix UploadPipeline/UploadPipelineVersion v2 API returns v1 object. #9064
Conversation
|
Thank you @chensun! /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gkcalat The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Since we have a test for UploadPipeline(), maybe we can add a test for UploadPipelineVersion() too?
There was a problem hiding this comment.
UploadPipelineVersion() is highly similar to UploadPipeline(), but that isn't a good excuse at all. I agree with you we should have better test coverage here. I will follow up on improving the test coverage shortly.
| if createdPipelineVersion == nil { | ||
| s.writeErrorToResponse(w, http.StatusInternalServerError, util.NewInternalServerError(errors.New("Failed to convert internal pipeline version representation to its v1beta1 API counterpart"), "Failed to create a pipeline version due to error converting it to API")) | ||
| if api_version == "v1beta1" { | ||
| err = marshaler.Marshal(w, toApiPipelineVersionV1(newPipelineVersion)) |
There was a problem hiding this comment.
In the previous code, we have a check for the output of toApiPipelineVersionV1() to be nil or not. I wonder if we should keep it.
There was a problem hiding this comment.
toApiPipelineVersionV1() per its current implementation never returns nil. So the previous code wasn't useful.
Description of your changes:
While integrating with v2 API, we noticed that UploadPipeline API always return v1 object (with resource references).
/cc @jlyaoyuli
Checklist: